-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cask/audit: allow dots in version and check unversioned cask #16882
Conversation
Signed-off-by: Michael Cho <[email protected]>
unversioned_token, _, version_designation = cask.token.rpartition("@") | ||
if unversioned_token.empty? | ||
match_data = /-(?<designation>alpha|beta|rc|release-candidate)$/.match(cask.token) | ||
if match_data && cask.tap&.official? && cask.tap != "homebrew/cask-versions" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if match_data && cask.tap&.official? && cask.tap != "homebrew/cask-versions" | |
if match_data && cask.tap&.official? && cask.tap.name != "homebrew/cask-versions" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering if we should only apply to homebrew/cask
given audit failures in fonts. From original location of check (which I'll probably move code back to later), I'm guessing we either don't apply that audit or there were no new fonts with -beta|alpha
.
As I recall, we have automation there that picks those names?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should definitely fix the automation, but maybe this should only apply to brew audit --new
for now?
add_error "unversioned cask token @ should be replaced by -at-" if unversioned_token.include? "@" | ||
|
||
if unversioned_token.match?(/[^a-z0-9-]/) | ||
add_error "unversioned cask token should only contain lowercase alphanumeric characters and hyphens" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add_error "unversioned cask token should only contain lowercase alphanumeric characters and hyphens" | |
add_error "cask token should only contain lowercase alphanumeric characters and hyphens" |
|
||
return if version_designation.empty? | ||
|
||
add_error "unversioned cask token should not have trailing hyphens" if unversioned_token.end_with?("-") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add_error "unversioned cask token should not have trailing hyphens" if unversioned_token.end_with?("-") | |
add_error "cask token should not have trailing hyphens" if unversioned_token.end_with?("-") |
version_designation = "" | ||
end | ||
|
||
add_error "unversioned cask token @ should be replaced by -at-" if unversioned_token.include? "@" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add_error "unversioned cask token @ should be replaced by -at-" if unversioned_token.include? "@" | |
add_error "cask token should use '-at-' instead of '@' in the unversioned part" if unversioned_token.include? "@" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I just realized that "unversioned cask" was poor phrasing given its existing usage for unversioned URLs (bump-unversioned-casks
).
"lowercase alphanumeric characters, hyphens and '.'" | ||
end | ||
|
||
if version_designation.start_with?("-", ".") || version_designation.end_with?(".") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if version_designation.start_with?("-", ".") || version_designation.end_with?(".") | |
if version_designation.start_with?("-", ".") || version_designation.end_with?("-", ".") |
end | ||
|
||
if version_designation.start_with?("-", ".") || version_designation.end_with?(".") | ||
add_error "cask token version designation should not have leading or trailing hyphens and/or '.'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add_error "cask token version designation should not have leading or trailing hyphens and/or '.'" | |
add_error "cask token version designation should not have leading or trailing hyphens and/or dots" |
|
||
if version_designation.match?(/[^.a-z0-9-]/) | ||
add_error "cask token version designation should only contain " \ | ||
"lowercase alphanumeric characters, hyphens and '.'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"lowercase alphanumeric characters, hyphens and '.'" | |
"lowercase alphanumeric characters, hyphens and dots" |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?This just polishes up some checks. Shouldn't block attempting migration so can be handled in parallel.